Stop the re-proposal loop; never write a blank feed row; make facts real - #110
Merged
Conversation
Three fixes, all found by running real work through the 0.5.0 runner against firstproof-c4 rather than by reading code. RE-PROPOSAL LOOP. A `decomposition` submit returns the parent to the pool — the proposal is inert until a peer approves it and the work still needs doing — but nothing stopped the parent being immediately re-claimed and re-proposed. That target accumulated EIGHT proposals of the same split, the agent's own summaries counting them off as "third pass", "fourth pass", "fifth pass"; two of those were my verification runs, at 35c and 21c. Each costs a volunteer's credit AND mints a review task costing a second volunteer, so the waste compounds on both sides — three of the five claimable tasks on that target are duplicate reviews of one split. The block lives at checkout, because that is where money is committed; refusing at submit would be too late, the run already happened. A task is blocked while a decomposition proposal of its own has a minted review task that nobody has contributed to yet. It lifts the moment the proposal is REVIEWED either way — approved (subtasks published) or rejected (the next agent gets the reviewer's reasons and can propose better). Keying on "has it been reviewed" rather than the review task's status matters: a rejected review sits in 'submitted' until verified, and gating on status left the parent stuck after its fate was already decided. Both pool listings hide such tasks too, so a runner never claims-and-bounces. BLANK SUMMARIES. An agent that omits `summary` wrote an empty string to the public feed — 8 of that target's contributions render as blank rows, including a real analytical result that cost 13c. Work units have had a synthesized fallback since v0.3.9; model tasks never did. submitResult now falls back to the same synthesis (task title plus a few headline scalars). The synthesizer moved to src/summary.ts: it is pure string work, and operations.ts runs in the Worker, so it cannot import workunit.ts (node:child_process). FACTS WERE INERT. #109 claimed the schema description meant the mechanism "doesn't ship inert". That was wrong — a JSON-schema field description is not an instruction, and the real delta_n run recorded zero facts while putting a genuine analytical result in `output` as usual. The system prompt now tells the agent what belongs in `facts` (anything the run settled, written to stand alone with its numbers in it; a ruled-out approach counts) and when to `$retract` (scaffolding describing work now done), and that state merges so it should never echo back the whole working set. 594 tests (7 new). Guard verified against a c4-shaped scenario: refused while unreviewed, hidden from both listings, claimable again once the review rules. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQgFEPRY4W74D4eqawQ4A6
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Barneyjm
added a commit
that referenced
this pull request
Aug 2, 2026
Four parallel review agents (reuse / simplification / efficiency / altitude) over the #109–#111 surface. Applied: LEAK. deriveNextSteps filtered only status='open' and was missing the two exclusions listAvailableTasks documents — onboarding_dev_id IS NULL and sensitivity = 'public'. getTargetProgress is unauthenticated, and the onboarding target is public and slugged, so next_steps.claimable would advertise per-dev onboarding tasks nobody else can claim, plus non-public tasks on public conjectures. They stay in the status counts; they never reach `claimable`. THE TOMBSTONE WAS THE WRONG SHAPE. boundedStateUpdate kept the byte TAIL of the serialized state under {truncated, note, tail} — not parseable JSON, so unreadable to the next agent, and an object of alien keys that then forced mergeStateUpdate to duck-type the marker so a later write wouldn't inherit it. Bounding is now key-aware: drop whole keys largest-first, name them in `_dropped`, and the stored value stays a real working set at every size. The isTombstone special case goes with its cause. Byte-preserving only mattered while established results lived in the blob; they live in target_facts now. `_dropped` is platform-owned and cleared on every merge, so one overflow cannot brand a working set forever. THE FIXED PATTERN HAD AN UNFIXED TWIN. salvageCrashedRun still spread the prior state back over itself, with a comment claiming it matched the timeout salvage — which #110 had changed. Both now send only their own key. EFFICIENCY. The per-fact INSERT loop ran one round trip per claim while holding FOR UPDATE on the targets row, with nothing capping how many claims an agent may send; now one unnest insert. getTargetProgress's three independent reads run concurrently. checkout folds the pending-decomposition predicate into the task SELECT it already runs, instead of a second round trip inside the money-path transaction. Migration 016 adds the two missing indexes: tasks(target_id, status) for the per-page-view scan, and a partial expression index for the review_of JSONB probe. TIDYING. deriveNextSteps counts in one pass, not five filters. The dead synthesizeWorkUnitSummary alias is gone. ParsedStateUpdate.state is typed `unknown`, which is what it always was. Skipped, with reasons in the reply: hydrating facts into checkout (real gap, own PR), facts/$retract as envelope siblings rather than reserved keys (interface decision), a real review_of column (larger migration), one shared isPlainObject (five files outside this diff). 595 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQgFEPRY4W74D4eqawQ4A6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three bugs found by running real work through the 0.5.0 runner against firstproof-c4 — not by reading code. Total verification spend: 69¢ across three tasks.
1. The re-proposal loop (costs money every time)
A
decompositionsubmit returns the parent to the pool: the proposal is inert until a peer approves it, and the work still needs doing. But nothing stopped the parent being re-claimed and re-proposed immediately.firstproof-c4's "Simulate slim(Δ)" has accumulated eight proposals of the same split — the agent's own summaries counting them off as "Phase-2 third pass", "fourth pass", "fifth pass". Two of those were my verification runs, at 35¢ and 21¢. Each redundant proposal burns a volunteer's credit and mints a review task that costs a second volunteer to dispose of, so the waste compounds on both sides: three of that target's five claimable tasks are now duplicate reviews of one split.
The block lives at checkout, because that's where money is committed — refusing at submit is too late, the run already happened. Both pool listings hide such tasks too, so a runner never claims-and-bounces.
The predicate keys on "has this proposal been reviewed", not on the review task's status. That distinction is load-bearing: a rejected review sits in
submitteduntil verified, and my first version gated on status, which left the parent stuck after its fate was already decided. Caught by the test asserting a rejected split reopens the parent.2. Blank feed rows
An agent that omits
summarywrote an empty string straight to the public feed. Eight of that target's contributions render as blank lines — including the δₙ analytical result that cost 13¢. Work units have had a synthesized fallback since v0.3.9; model tasks never did.submitResultnow falls back to the same synthesis (task title + a few headline scalars). The synthesizer moved tosrc/summary.ts— it's pure string work, andoperations.tsruns in the Worker, so it can't importworkunit.ts(node:child_process).3.
factswas inert — my error in #109#109 claimed the schema description meant the mechanism "doesn't ship inert." That was wrong. A JSON-schema field description is not an instruction.
factsappeared exactly once inexecutor.tsand nowhere in the prompt, and the real δₙ run recorded zero facts while putting a genuine analytical result inoutputas usual.The system prompt now tells the agent what belongs in
facts(anything the run settled, written to stand alone with its numbers in it — a ruled-out approach counts), when to$retract(scaffolding describing work now done), and that state merges per key so it should never echo the whole working set back.Verified
594 tests, 7 new. The guard checked against a C4-shaped scenario:
What #109 got right, confirmed against production across three real submits:
phase2,attempt7_noteandtimeout_salvagesurvived all of them completely unchanged. Under the old replacement semantics any one of those submits could have wiped the lot.Not in this PR
No version bump — but note
src/executor.tschanged, so this does alter the published runner and will need one before release. I've left it at 0.5.0 since v0.5.0 was never tagged; the bump rides whichever PR is last before you cut it.Still open: the informativeness guard (nothing refuses a task whose success criterion is trivially satisfiable — C4's actual mathematical failure), and surfacing
facts/next_stepson the conjecture page.🤖 Generated with Claude Code
https://claude.ai/code/session_01QQgFEPRY4W74D4eqawQ4A6